Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-36863: [C#][Packaging] Do not shutdown PythonEngine on CDataInterfacePythonTests if .NET is > 5.0 #36868

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Jul 25, 2023

Rationale for this change

Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue.

What changes are included in this PR?

Only Shutdown if #if !NET5_0_OR_GREATER

Are these changes tested?

Locally and via archery.

Are there any user-facing changes?

No

@raulcd raulcd requested a review from westonpace as a code owner July 25, 2023 13:24
@github-actions
Copy link

⚠️ GitHub issue #36863 has been automatically assigned in GitHub to PR creator.

@raulcd
Copy link
Member Author

raulcd commented Jul 25, 2023

@github-actions crossbow submit nuget

@github-actions
Copy link

Revision: a6e6d4e

Submitted crossbow builds: ursacomputing/crossbow @ actions-21edeec61b

Task Status
nuget Github Actions

@raulcd
Copy link
Member Author

raulcd commented Jul 25, 2023

@westonpace are you ok with this?

@westonpace
Copy link
Member

@CurtHagenlocher does this make sense to you? I recall you mentioning shutting down python but now I can't find that comment.

@CurtHagenlocher
Copy link
Contributor

The unload is only required when not using .NET Core. Under .NET Framework, unloading the AppDomain hangs when Python.NET hasn't previously been unloaded (and Core doesn't have AppDomains).

Disabling this for Core runs returns us to status quo and avoids a mysterious crash we were seeing in the release branch.

@westonpace
Copy link
Member

Ah, I see your comments on the original issue too! I am fine with this change.

@raulcd raulcd merged commit d9b9003 into apache:main Jul 25, 2023
9 checks passed
@raulcd raulcd removed the awaiting committer review Awaiting committer review label Jul 25, 2023
raulcd added a commit that referenced this pull request Jul 25, 2023
…acePythonTests if .NET is > 5.0 (#36868)

### Rationale for this change

Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue.

### What changes are included in this PR?

Only Shutdown if `#if !NET5_0_OR_GREATER`

### Are these changes tested?

Locally and via archery.

### Are there any user-facing changes?

No
* Closes: #36863

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit d9b9003.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this pull request Aug 20, 2023
…InterfacePythonTests if .NET is > 5.0 (apache#36868)

### Rationale for this change

Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue.

### What changes are included in this PR?

Only Shutdown if `#if !NET5_0_OR_GREATER`

### Are these changes tested?

Locally and via archery.

### Are there any user-facing changes?

No
* Closes: apache#36863

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…InterfacePythonTests if .NET is > 5.0 (apache#36868)

### Rationale for this change

Tests are failing on maintenance branch to generate Nuget packages. This has been tested on the maintenance branch and it solves the issue.

### What changes are included in this PR?

Only Shutdown if `#if !NET5_0_OR_GREATER`

### Are these changes tested?

Locally and via archery.

### Are there any user-facing changes?

No
* Closes: apache#36863

Authored-by: Raúl Cumplido <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Packaging][C#] Nuget build is failing on maintenance branch for 13.0.0
3 participants